tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg #137167

martn3 · 2025-02-17T09:21:33Z

In #136324 the doctest for f16::erf() was gated with reliable_f16_math. Add the same gate on f16::erfc() to avoid:

rust_out.71e2e529d20ea47d-cgu.0:\
(.text._ZN8rust_out4main43_doctest_main_library_std_src_f16_rs_1321_017h485f3ffe6bf2a981E+0x38): \
undefined reference to `__gnu_h2f_ieee'

on MIPS (and maybe other architectures).

r? tgross35

rustbot · 2025-02-17T09:21:43Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @tgross35 (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

In 136324 the doctest for `f16::erf()` was gated with `reliable_f16_math`. Add the same gate on `f16::erfc()` to avoid: rust_out.71e2e529d20ea47d-cgu.0:\ (.text._ZN8rust_out4main43_doctest_main_library_std_src_f16_rs_1321_017h485f3ffe6bf2a981E+0x38): \ undefined reference to `__gnu_h2f_ieee' on MIPS (and maybe other architectures).

GrigorenkoPV

Sorry, missed this one. I've double-checked now, and looks like this indeed is the only one that's missing.

tgross35 · 2025-02-17T19:16:16Z

@bors r+ rollup

bors · 2025-02-17T19:16:19Z

📌 Commit f6485ff has been approved by tgross35

It is now in the queue for this repository.

tgross35 · 2025-02-17T19:35:49Z

Thanks for the catch, unfortunately I don't think we run tests on any platforms that don't support f16

rust/library/std/build.rs

Lines 97 to 118 in ce36a96

    
           let has_reliable_f16 = match (target_arch.as_str(), target_os.as_str()) { 
        
               // We can always enable these in Miri as that is not affected by codegen bugs. 
        
               _ if is_miri => true, 
        
               // Selection failure <https://github.com/llvm/llvm-project/issues/50374> 
        
               ("s390x", _) => false, 
        
               // Unsupported <https://github.com/llvm/llvm-project/issues/94434> 
        
               ("arm64ec", _) => false, 
        
               // MinGW ABI bugs <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115054> 
        
               ("x86_64", "windows") if target_env == "gnu" && target_abi != "llvm" => false, 
        
               // Infinite recursion <https://github.com/llvm/llvm-project/issues/97981> 
        
               ("csky", _) => false, 
        
               ("hexagon", _) => false, 
        
               ("loongarch64", _) => false, 
        
               ("mips" | "mips64" | "mips32r6" | "mips64r6", _) => false, 
        
               ("powerpc" | "powerpc64", _) => false, 
        
               ("sparc" | "sparc64", _) => false, 
        
               ("wasm32" | "wasm64", _) => false, 
        
               // `f16` support only requires that symbols converting to and from `f32` are available. We 
        
               // provide these in `compiler-builtins`, so `f16` should be available on all platforms that 
        
               // do not have other ABI issues or LLVM crashes. 
        
               _ => true, 
        
           };

. s390x has a dist job but that doesn't run anything.

However, I think we could actually remove mips from that list if llvm/llvm-project#110199 made it into llvm 20, which Rust just upgraded to. @martn3 if that sounds right, you could try deleting https://github.com/rust-lang/compiler-builtins/blob/3c09866aa4c699ed5f430567f6de97ed56b65126/configure.rs#L81 and then running tests. If it works also update the above std/build.rs, and then we have f16 on mips 🎉

… r=tgross35 tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg In rust-lang#136324 the doctest for `f16::erf()` was gated with `reliable_f16_math`. Add the same gate on `f16::erfc()` to avoid: rust_out.71e2e529d20ea47d-cgu.0:\ (.text._ZN8rust_out4main43_doctest_main_library_std_src_f16_rs_1321_017h485f3ffe6bf2a981E+0x38): \ undefined reference to `__gnu_h2f_ieee' on MIPS (and maybe other architectures). r? tgross35

…llaumeGomez Rollup of 11 pull requests Successful merges: - rust-lang#127793 (Added project-specific Zed IDE settings) - rust-lang#134995 (Stabilize const_slice_flatten) - rust-lang#135767 (Future incompatibility warning `unsupported_fn_ptr_calling_conventions`: Also warn in dependencies) - rust-lang#136599 (librustdoc: more usages of `Joined::joined`) - rust-lang#136750 (Make ub_check message clear that it's not an assert) - rust-lang#137000 (Deeply normalize item bounds in new solver) - rust-lang#137126 (fix docs for inherent str constructors) - rust-lang#137151 (Install more signal stack trace handlers) - rust-lang#137161 (Pattern Migration 2024: fix incorrect messages/suggestions when errors arise in macro expansions) - rust-lang#137167 (tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg) - rust-lang#137177 (Update `minifier-rs` version to `0.3.5`) r? `@ghost` `@rustbot` modify labels: rollup

… r=tgross35 tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg In rust-lang#136324 the doctest for `f16::erf()` was gated with `reliable_f16_math`. Add the same gate on `f16::erfc()` to avoid: rust_out.71e2e529d20ea47d-cgu.0:\ (.text._ZN8rust_out4main43_doctest_main_library_std_src_f16_rs_1321_017h485f3ffe6bf2a981E+0x38): \ undefined reference to `__gnu_h2f_ieee' on MIPS (and maybe other architectures). r? tgross35

Rollup of 9 pull requests Successful merges: - rust-lang#137151 (Install more signal stack trace handlers) - rust-lang#137167 (tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg) - rust-lang#137195 (cg_clif: use exclusively ABI alignment) - rust-lang#137202 (Enforce T: Hash for Interned<...>) - rust-lang#137205 (Remove `std::os::wasi::fs::FileExt::tell`) - rust-lang#137211 (don't ICE for alias-relate goals with error term) - rust-lang#137213 (Remove `rustc_middle::mir::tcx` module.) - rust-lang#137214 (add last std diagnostic items for clippy) - rust-lang#137221 (Remove scrutinee_hir_id from ExprKind::Match) r? `@ghost` `@rustbot` modify labels: rollup

… r=tgross35 tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg In rust-lang#136324 the doctest for `f16::erf()` was gated with `reliable_f16_math`. Add the same gate on `f16::erfc()` to avoid: rust_out.71e2e529d20ea47d-cgu.0:\ (.text._ZN8rust_out4main43_doctest_main_library_std_src_f16_rs_1321_017h485f3ffe6bf2a981E+0x38): \ undefined reference to `__gnu_h2f_ieee' on MIPS (and maybe other architectures). r? tgross35

Rollup of 9 pull requests Successful merges: - rust-lang#136750 (Make ub_check message clear that it's not an assert) - rust-lang#137151 (Install more signal stack trace handlers) - rust-lang#137167 (tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg) - rust-lang#137195 (cg_clif: use exclusively ABI alignment) - rust-lang#137202 (Enforce T: Hash for Interned<...>) - rust-lang#137205 (Remove `std::os::wasi::fs::FileExt::tell`) - rust-lang#137211 (don't ICE for alias-relate goals with error term) - rust-lang#137214 (add last std diagnostic items for clippy) - rust-lang#137221 (Remove scrutinee_hir_id from ExprKind::Match) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#137167 - martn3:reliable_f16_math-f16-erfc, r=tgross35 tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg In rust-lang#136324 the doctest for `f16::erf()` was gated with `reliable_f16_math`. Add the same gate on `f16::erfc()` to avoid: rust_out.71e2e529d20ea47d-cgu.0:\ (.text._ZN8rust_out4main43_doctest_main_library_std_src_f16_rs_1321_017h485f3ffe6bf2a981E+0x38): \ undefined reference to `__gnu_h2f_ieee' on MIPS (and maybe other architectures). r? tgross35

martn3 · 2025-02-19T09:04:14Z

However, I think we could actually remove mips from that list if llvm/llvm-project#110199 made it into llvm 20, which Rust just upgraded to. @martn3 if that sounds right, you could try deleting https://github.com/rust-lang/compiler-builtins/blob/3c09866aa4c699ed5f430567f6de97ed56b65126/configure.rs#L81 and then running tests. If it works also update the above std/build.rs, and then we have f16 on mips 🎉

@tgross35 According to my testing, f16 works on MIPS with Rust master branch with LLVM 20 🎉 . I ran with Rust master plus this:

diff --git a/configure.rs b/configure.rs
index fa3e302..ff52e88 100644
--- a/configure.rs
+++ b/configure.rs
@@ -80,3 +80,2 @@ pub fn configure_f16_f128(target: &Target) {
         "loongarch64" => false,
-        "mips" | "mips64" | "mips32r6" | "mips64r6" => false,
         "powerpc" | "powerpc64" => false,

and this:

diff --git a/library/Cargo.lock b/library/Cargo.lock
index 0be2f9a1549..5340d74fc3c 100644
--- a/library/Cargo.lock
+++ b/library/Cargo.lock
@@ -64,4 +64,3 @@ name = "compiler_builtins"
 version = "0.1.146"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a97117b1434b79833f39a5fabdf82f890bd98c1988334dea1cb67f7e627fa311"
+source = "git+https://github.com/martn3/compiler-builtins.git?branch=f16-mips#b15a085cf1ca4b8e9d31b56dd564bb8954306ae5"
 dependencies = [
diff --git a/library/Cargo.toml b/library/Cargo.toml
index 1205f7c9ed6..b973d04d99e 100644
--- a/library/Cargo.toml
+++ b/library/Cargo.toml
@@ -43,2 +43,3 @@ rustc-demangle.debug = 0
 [patch.crates-io]
+compiler_builtins = { git = 'https://github.com/martn3/compiler-builtins.git', branch = "f16-mips" }
 # See comments in `library/rustc-std-workspace-core/README.md` for what's going on
diff --git a/library/std/build.rs b/library/std/build.rs
index 8dc326a3dde..723d1eb02e0 100644
--- a/library/std/build.rs
+++ b/library/std/build.rs
@@ -109,3 +109,2 @@ fn main() {
         ("loongarch64", _) => false,
-        ("mips" | "mips64" | "mips32r6" | "mips64r6", _) => false,
         ("powerpc" | "powerpc64", _) => false,

Disclaimer: I only tested on the MIPS hardware that happens to be available to me, and can't say anything about other MIPS hardware.

Disclaimer: I didn't run tests that depend on conformant NaNs due to MIPS having non-conformat NaNs. In particular I skipped these tests: f16::f16::powf, f32::f32::powf, f64::f64::powf, SimdFloat::reduce_max, SimdFloat::reduce_min, iter::traits::iterator::Iterator::max, test_min_max_nan.

tgross35 · 2025-02-19T18:03:02Z

That's great news! Feel free to put up PRs to make those changes and r? me.

Disclaimer: I didn't run tests that depend on conformant NaNs due to MIPS having non-conformat NaNs. In particular I skipped these tests: f16::f16::powf, f32::f32::powf, f64::f64::powf, SimdFloat::reduce_max, SimdFloat::reduce_min, iter::traits::iterator::Iterator::max, test_min_max_nan.

If those tests rely on total_cmp ordering between sNaNs and qNaNs, it may be feasible to adjust the implementations to flip the quieting bit on relevant targets.

Enable `f16` for MIPS Blocked on rust-lang/compiler-builtins#762 It seems as if `f16` works on MIPS now according to my testing on Rust master with LLVM 20, and I was asked [here](rust-lang#137167 (comment)) to create PRs with my changes. I only tested on the flavour of `mipsel-unknown-linux-gnu` hardware that happens to be available to me, so I can't say anything about other MIPS hardware, but from a casual skimming of the LLVM code ([1], [2]) it seems like `f16` should work on all MIPS hardware. So enable it for all MIPS hardware. [1]: https://github.com/rust-lang/llvm-project/blob/rustc/20.1-2025-02-13/llvm/lib/Target/Mips/MipsISelLowering.h#L370 [2]: https://github.com/rust-lang/llvm-project/blob/rustc/20.1-2025-02-13/llvm/lib/CodeGen/TargetLoweringBase.cpp#L1367-L1388 `@rustbot` label +O-MIPS +F-f16_and_f128 +S-blocked Tracking issue for f16: rust-lang#116909 r? `@tgross35`

Rollup merge of rust-lang#137311 - martn3:enable-f16-mips, r=tgross35 Enable `f16` for MIPS Blocked on rust-lang/compiler-builtins#762 It seems as if `f16` works on MIPS now according to my testing on Rust master with LLVM 20, and I was asked [here](rust-lang#137167 (comment)) to create PRs with my changes. I only tested on the flavour of `mipsel-unknown-linux-gnu` hardware that happens to be available to me, so I can't say anything about other MIPS hardware, but from a casual skimming of the LLVM code ([1], [2]) it seems like `f16` should work on all MIPS hardware. So enable it for all MIPS hardware. [1]: https://github.com/rust-lang/llvm-project/blob/rustc/20.1-2025-02-13/llvm/lib/Target/Mips/MipsISelLowering.h#L370 [2]: https://github.com/rust-lang/llvm-project/blob/rustc/20.1-2025-02-13/llvm/lib/CodeGen/TargetLoweringBase.cpp#L1367-L1388 `@rustbot` label +O-MIPS +F-f16_and_f128 +S-blocked Tracking issue for f16: rust-lang#116909 r? `@tgross35`

rustbot assigned tgross35 Feb 17, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 17, 2025

martn3 force-pushed the reliable_f16_math-f16-erfc branch from 799639b to f6485ff Compare February 17, 2025 10:59

martn3 changed the title ~~std: Also gate f16::erfc() with reliable_f16_math cfg~~ tests: Also gate f16::erfc() doctest with reliable_f16_math cfg Feb 17, 2025

GrigorenkoPV approved these changes Feb 17, 2025

View reviewed changes

tgross35 approved these changes Feb 17, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 17, 2025

GuillaumeGomez mentioned this pull request Feb 18, 2025

Rollup of 11 pull requests #137212

Closed

Urgau mentioned this pull request Feb 18, 2025

Rollup of 9 pull requests #137226

Closed

Urgau mentioned this pull request Feb 18, 2025

Rollup of 9 pull requests #137231

Merged

matthiaskrgr mentioned this pull request Feb 18, 2025

Rollup of 11 pull requests #137234

Closed

bors merged commit 14fb84a into rust-lang:master Feb 18, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Feb 18, 2025

This comment was marked as outdated.

Sign in to view

This was referenced Feb 20, 2025

Enable f16 for MIPS rust-lang/compiler-builtins#762

Merged

Enable f16 for MIPS #137311

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg #137167

tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg #137167

martn3 commented Feb 17, 2025

rustbot commented Feb 17, 2025

GrigorenkoPV left a comment

tgross35 commented Feb 17, 2025

bors commented Feb 17, 2025

tgross35 commented Feb 17, 2025

martn3 commented Feb 19, 2025 •

edited

Loading

tgross35 commented Feb 19, 2025 •

edited

Loading

This comment was marked as outdated.

tests: Also gate f16::erfc() doctest with reliable_f16_math cfg #137167

tests: Also gate f16::erfc() doctest with reliable_f16_math cfg #137167

Conversation

martn3 commented Feb 17, 2025

rustbot commented Feb 17, 2025

GrigorenkoPV left a comment

Choose a reason for hiding this comment

tgross35 commented Feb 17, 2025

bors commented Feb 17, 2025

tgross35 commented Feb 17, 2025

martn3 commented Feb 19, 2025 • edited Loading

tgross35 commented Feb 19, 2025 • edited Loading

This comment was marked as outdated.

tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg #137167

tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg #137167

martn3 commented Feb 19, 2025 •

edited

Loading

tgross35 commented Feb 19, 2025 •

edited

Loading